Skip to content

Stage the styling contract: compatibility= modes and the engine collision table - #453

Closed
Alek99 wants to merge 3 commits into
alek/compat-1-preflightfrom
alek/compat-2-modes
Closed

Stage the styling contract: compatibility= modes and the engine collision table#453
Alek99 wants to merge 3 commits into
alek/compat-1-preflightfrom
alek/compat-2-modes

Conversation

@Alek99

@Alek99 Alek99 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Stacked on #452 (preflight report + applicable-slot matrix) — second change of the styling-compatibility program's Phase 0.

What this adds

Every image-export API (to_png, to_svg, to_image, write_image, batch write_images — at chart, figure, and module level) accepts compatibility=:

Mode Behavior
"legacy" (default) Exactly today's behavior — byte-identical output, tested.
"warn" One StyleCompatibilityWarning naming each declaration the export would drop; bytes still produced.
"strict" StyleCompatibilityError before emission, full preflight report attached, remedies in the message.
"lossless" Reserved and rejected until Engine.auto can actually re-route on preflight evidence — accepting the name earlier would make it a lie.

The engine contract

Engine selection and compatibility are orthogonal; an explicit engine is a hard constraint no mode may override:

  • strict + pinned native engine → refuses on that engine, never silently re-routes to Chromium;
  • any mode + Chromium engine → the browser renders the full cascade, nothing can drop, the mode has nothing to do (tested against a live Chromium);
  • resolution errors (custom_css + pinned native, Chromium SVG) precede and outrank mode logic in every mode — today's ValueErrors fire unchanged;
  • a strict batch fails whole while the plan is resolved, before any file is written;
  • HTML rejects a non-default mode like the other options it cannot honor.

State-gated chrome never trips warn/strict in a clean static export (the applicable-slot contract from #452, now enforced rather than only reported).

Performance posture

  • The default path pays one string comparison: literal-legacy returns before the preflight machinery is imported.
  • xy.export's StyleCompatibilityError/Warning aliases resolve via module __getattr__, so importing the module stays exactly as heavy as before — verified that the eager alias would have pulled the native dylib into an import chain that never had it, and rejected for that reason.
  • warn/strict pay the routing walk only for charts carrying class or per-slot declarations on a native engine.

Migration schedule (named, per Phase 0's rule)

spec/process/style-compatibility-migration.md: compatibility= ships default-legacy in 0.0.6 → default flips to warn in 0.0.7 → strict at the 0.1.0 minor boundary → legacy removed in 0.2.0. Facet grids deliberately don't accept the option yet (tracked there) rather than half-honoring it.

Evidence

14 new mode tests covering the engine × mode matrix (including live-Chromium runs); the one adjusted existing test is the Chart→Figure delegation contract, extended for the grown kwarg. Full suite: 3987 passed, 109 skipped. ruff/format/pre-commit clean; xy.export import weight verified unchanged.

Review in cubic

…sion table

Every image-export API (to_png, to_svg, to_image, write_image, and the
write_images batch, at chart, figure, and module level) accepts
compatibility=: "legacy" preserves today's behavior exactly, "warn" surfaces
every declaration the export would drop as one StyleCompatibilityWarning,
and "strict" raises StyleCompatibilityError before emission with the full
preflight report attached. "lossless" is reserved and rejected: accepting
the name before Engine.auto can actually re-route on preflight evidence
would make it a lie.

The default path pays one string comparison. The literal-"legacy"
short-circuit returns before the preflight machinery is even imported, and
export's StyleCompatibilityError/Warning aliases resolve through a module
__getattr__, so importing xy.export stays exactly as heavy as it was before
the modes existed (the preflight chain reaches the native library through
the writers' constants — eagerly aliasing it would have added the dylib
load to a module import that never had one). warn/strict pay the routing
walk only when the chart carries class or per-slot declarations and the
engine is native.

Engine selection and compatibility stay orthogonal, and an explicit engine
is a hard constraint: strict refuses on the pinned engine rather than
re-routing to Chromium, a Chromium pin renders the full cascade and gives
every mode nothing to do, and resolution errors (custom_css with a pinned
native engine, Chromium SVG) precede and outrank mode logic in every mode.
A strict batch fails whole while the plan is resolved, before any file is
written. HTML export rejects a non-default mode like the other options it
cannot honor, because a document that renders the full cascade has nothing
to check.

State-gated chrome never trips warn or strict in a clean static export: a
file with no tooltip has dropped nothing by not styling one — the
applicable-slot contract from the preflight change, now enforced rather
than only reported.

spec/process/style-compatibility-migration.md names the release each
default flips in — warn in 0.0.7, strict at the 0.1.0 minor boundary,
legacy removed in 0.2.0 — per the rule that a deprecation window is named
when it opens, not "a future release". Facet grids deliberately do not
accept the option yet rather than half-honoring it; their per-panel
preflight is tracked there too.
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 684e949f-e71f-4db0-8455-2264ed137998

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR stages compatibility modes across chart-, figure-, and module-level image export APIs while preserving legacy behavior by default.

  • Adds warn and strict handling backed by styling preflight reports.
  • Preserves engine selection as a separate hard constraint and rejects the reserved lossless mode.
  • Adds batch planning behavior, public aliases, migration documentation, and mode-matrix tests.

Confidence Score: 4/5

The PR does not yet appear safe to merge because batch HTML export still silently accepts and ignores non-default compatibility modes.

The previous to_png engine-validation issue is fixed, but write_images still bypasses non-default compatibility rejection for HTML entries while the equivalent write_image path raises, leaving the previously reported API inconsistency outstanding.

Files Needing Attention: python/xy/export.py, tests/test_style_compatibility_modes.py

Important Files Changed

Filename Overview
python/xy/export.py Adds compatibility validation and enforcement throughout export routing; the prior to_png engine-validation ordering issue is fixed, but the previously reported batch HTML inconsistency remains.
python/xy/styling/preflight.py Adds mode validation, warning/error types, resolved-engine enforcement, and caller-oriented warning attribution.
python/xy/_figure.py Threads compatibility through Figure image-export methods and enforces it for native SVG export.
python/xy/components.py Exposes and forwards compatibility through Chart export wrappers while retaining export-config default handling.
tests/test_style_compatibility_modes.py Covers the engine-by-mode contract and confirms the prior to_png ordering fix, while explicitly preserving the outstanding batch HTML behavior.
spec/api/export.md Documents compatibility modes, engine constraints, and styling survival across export families.
spec/process/style-compatibility-migration.md Defines the staged default changes and eventual removal schedule for legacy mode.

Reviews (2): Last reviewed commit: "Review follow-ups: precedence, attributi..." | Re-trigger Greptile

Comment thread python/xy/export.py Outdated
@codspeed-hq

codspeed-hq Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 109 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing alek/compat-2-modes (10b5030) with alek/compat-1-preflight (f661794)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 9 files

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread CHANGELOG.md Outdated
Comment thread python/xy/export.py
Comment thread tests/test_style_compatibility_modes.py
Comment thread tests/test_style_compatibility_modes.py Outdated
Comment thread python/xy/styling/preflight.py Outdated
Comment thread python/xy/styling/preflight.py Outdated
Comment thread python/xy/export.py
Alek99 added 2 commits August 4, 2026 13:48
…cabulary

Resolution errors now precede mode logic in to_png for styled charts too:
the custom_css/native refusal is checked right after engine resolution,
before enforcement can warn or raise, so it stays the documented ValueError
in every mode. The precedence test now feeds a lossy chart — the case that
previously slipped past it — and asserts the error is not the strict
subclass.

Compatibility warnings land on the caller's export line. The distance from
the warn call to user code varies by entry point (module function, Figure
method, Chart method), so the stacklevel is measured: walk outward from
enforce to the first frame outside the package. A test pins the attribution
for three different entry points.

Strict SVG failures no longer recommend engine=Engine.chromium, which that
format rejects; the remediation is format-aware. write_images validates the
compatibility vocabulary once up front, so a typo fails even an all-HTML
batch instead of passing silently — while HTML entries themselves stay
exempt from the mode, since a document that renders the full cascade has
nothing to check. The changelog entry now names the facet-grid exclusion
instead of overstating coverage, and the raster-drops-vector-only test is
named for what it exercises.
@Alek99

Alek99 commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Consolidated into #455 (single-PR review requested) — same commits including the review follow-ups; the finding dispositions are listed in #455's description.

@Alek99 Alek99 closed this Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant